improvement: ignore create or modify file watcher event when content did not change #7006
Conversation
| event.eventType match { | ||
| case EventType.CreateOrModify => | ||
| try { | ||
| val md5 = MD5.bytesToHex(FileIO.readAllBytes(path)) |
There was a problem hiding this comment.
Later in onChange the file is read again (if the event isn't ignored). Should I try to deduplicate these reads?
| event.eventType match { | ||
| case EventType.CreateOrModify => | ||
| try { | ||
| val md5 = MD5.bytesToHex(FileIO.readAllBytes(path)) |
There was a problem hiding this comment.
Maybe we should do it before compile? So that we wouldn't do noop compilations as well?
There was a problem hiding this comment.
I put it before compile. I also added a draft of a solution to deduplicate reading in file content. (I feel like that might be costly.) WDYT?
There was a problem hiding this comment.
I realised we also compile the file when it did not change but its dependencies did (on didFocus). I wonder if it's safe to always discard compile on didSave if the file did not change. Can we rely only on didFocus? Otherwise, there may be some compilation that finished and user may want to force compile (on this build target) using save. Maybe the first option to only deduplicate on file watcher events was better?
There was a problem hiding this comment.
I realised we also compile the file when it did not change but its dependencies did (on didFocus). I wonder if it's safe to always discard compile on didSave if the file did not change.
But if they want to force a save with didSave, they need to change the file, right? Did save will not be sent on a unchanged file anyway.
If any upstream projects change, we should always maybe mark anything that depends on them as "dirty" and should always be recompiled next time a compilation is invoked?
There was a problem hiding this comment.
But if they want to force a save with didSave, they need to change the file, right? Did save will not be sent on a unchanged file anyway
At least in VSCode Save will always send didSave even if the file did not change.
There was a problem hiding this comment.
I don't see didSave being sent if I save an unchanged file 🤔 Or do you mean in a different case?
| focusedDocumentBuildTarget: Option[BuildTargetIdentifier], | ||
| ): Future[Unit] = { | ||
| val paths = |
There was a problem hiding this comment.
Maybe we could just find first that changed in a given target?
There was a problem hiding this comment.
Yes, though I doubt it ever too many files at once. Only file watcher passes more than one file.
| import scala.meta.io.AbsolutePath | ||
|
|
||
| class SavedFileSignatures { | ||
| private val previousCreateOrModify = TrieMap[AbsolutePath, String]() |
There was a problem hiding this comment.
Could we reuse MutableMd5Fingerprints somehow? Maybe just save them in didSave with a flag to MutableMd5Fingerprints and then we can get the last saved one instead of calculating again?
We can still have a separate collection in Compilations so that it's possible to clear it and force new compilations?
There was a problem hiding this comment.
For didSave we put them anyway there but for file watcher event like CreateOrModify we don't, and we don't want to since it stores content, which we don't need. Also there is no way to check that the caller of compileFile/s saved the fingerprint (there may be an old one there), so it becomes some hidden knowledge that you should.
Edit: we can potentially use flags and mark "used ones"
fixes: #6896
Problem
Code generation leads to infinite compilation loop.
Solution
Ignore create or modify file watcher event when content of the file is the same as with the previously when this event was triggered.Ignore compile for file if it did not change from last compile.